Wait for pod template on resume to avoid leaking agent pods after restart#2851
Wait for pod template on resume to avoid leaking agent pods after restart#2851RanjitCS wants to merge 8 commits into
Conversation
| private static final Logger LOGGER = Logger.getLogger(KubernetesLauncher.class.getName()); | ||
|
|
||
| private static final long TEMPLATE_RESOLUTION_TIMEOUT_SECONDS = | ||
| SystemProperties.getLong(KubernetesLauncher.class.getName() + ".templateResolutionTimeoutSeconds", 60L); |
There was a problem hiding this comment.
Use getDuration please, and remove the time unit from the property name, and adjust docs to clarify that you pass a unit expression.
| * re-registers the dynamic template asynchronously, and this launcher may run first. Rather than failing | ||
| * immediately (which would leak the pod), poll for the template for a short while. Falls back to | ||
| * {@link KubernetesSlave#getTemplate()} (which throws a descriptive exception) if it never becomes available. | ||
| * See <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a>. |
There was a problem hiding this comment.
| * See <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a>. |
Anyone interested can git blame.
| * See <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a>. | ||
| */ | ||
| @NonNull | ||
| private static PodTemplate waitForTemplate(KubernetesSlave node) throws InterruptedException { |
There was a problem hiding this comment.
Should this not be waiting only for dynamic templates? A global template would already be available, or should be, so if it is not then we may as well fail immediately.
| LOGGER.log( | ||
| FINE, |
There was a problem hiding this comment.
| LOGGER.log( | |
| FINE, | |
| LOGGER.fine( |
| * launch and restores it shortly after. Without the fix the pod is orphaned and with it the build resumes on the | ||
| * same pod and the pod is cleaned up. | ||
| */ | ||
| @Issue("JENKINS-67390") |
There was a problem hiding this comment.
Jira is dead. Refer to GH Issues
| } | ||
|
|
||
| /** | ||
| * Reproduces <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a> if, on resume, |
There was a problem hiding this comment.
| * Reproduces <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a> if, on resume, | |
| * Reproduces a problem in which, on resume, |
| cloud.removeDynamicTemplate(template); | ||
| // Restore it shortly after, mimicking PodTemplateStepExecution#onResume re-registering it late. | ||
| Timer.get().schedule(() -> cloud.addDynamicTemplate(template), 5, TimeUnit.SECONDS); |
There was a problem hiding this comment.
This is not very convincing. If there is no way to reliably reproduce the actual problem, I would rather not have a test at all.
There was a problem hiding this comment.
as its a very narrow window, the initial test which i had was more faithful but was flaky without the fix (i.e 4/10 failures without the fix, fully passes with the fix), i will remove the test itself then if this is not good enough
| containerTemplate(name: 'busybox', image: 'busybox', command: 'cat', ttyEnabled: true) | ||
| ]) { | ||
| node('orphan-race') { | ||
| container('busybox') { |
There was a problem hiding this comment.
Is the container step relevant in this context?
| @@ -0,0 +1,15 @@ | |||
| podTemplate(label: 'orphan-race', yaml: ''' | |||
There was a problem hiding this comment.
do not use label, long deprecated
Details? |
thinking it through, as i found the same symptom (suspended agent, pod never cleaned up) while trying to reproduce, i assumed it was the same fix, but i was wrong, i will continue to focus on #2512, but i believe this is also a bug that can happen edit: i have spent some time trying to reproduce #2512, but i have been unsuccessfull, i believe #1137 has solved the "agent stays suspended, pod reconnects but Jenkins does nothing" problem. this pr however is still valid |
again, this is extremely timing dependent but can be reproduced consistently without any fake pause injection at least 5/10 tries i did |
If the controller restarts while a dynamic pod template agent is being provisioned,
KubernetesLauncher.launch()fails on resume because the template isnt re-registered yet, leaving the pod orphanedTesting done
Submitter checklist